Update finetuned_regressor.py#772
Conversation
Pass through kwargs, e.g. needed for finetuned_reg.predict(X_test, output_type='full')
jonaslandsgesell
requested review from
adrian-prior
and removed request for
a team
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Jonas Landsgesell seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for passing keyword arguments to the predict method of FinetunedTabPFNRegressor, which is a good enhancement for flexibility. My review focuses on ensuring the type hints are updated to reflect the new capabilities. I've identified that the return type of predict is now incorrect given the new arguments and have suggested a fix to make it accurate.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
@jonaslandsgesell Thanks for the PR! The changes look correct to me, but you can see in the failing tests that there are a few things missing (changelog, formatting, etc.). Please add these and them I happy to give it a proper review :) |
|
@jonaslandsgesell Also, have you already signed the CLA? See the bot commenting above. In addition, I think you also might have incorrectly formatted the changelog (see the changelog test on how to correctly add that) |
jonaslandsgesell
force-pushed
the
patch-1
branch
from
fed17b4 to
d0a04ab
Compare
|
@jonaslandsgesell Have you already signed the CLA? The test still indicates that it hasn't been signed. |
Yes I signed with this GitHub account. |
adrian-prior
left a comment
adrian-prior
left a comment
There was a problem hiding this comment.
Generally, the changes look good to me. Please also modify predict and predict_prob in the FinetunedClassifier accordingly. Regarding the CLA:
I think some of the commits aren't associated with your GitHub Account. Hopefully, I can merge the PR without the CLA test passing. Otherwise, there seems to be an option to add the correct email in retrospect.
|
As far as I see, the predict and predict_proba method in the TabPFNClassifier do not seem to currently take additional keyword arguments. Do you want me to still add the kwargs parameters? |
|
@jonaslandsgesell Good spot. I would still go for it. Could very well be the case that this will change soon :) |
3 participants
Pass through kwargs,
Motivation and Context
e.g. needed for finetuned_reg.predict(X_test, output_type='full')
Public API Changes
Checklist
changelog/README.md), or "no changelog needed" label requested.